-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Don't marshal empty byte or uint8 slice as null #371
Conversation
cc @sttts |
@nikhita i was playing with this snippet - https://play.golang.org/p/IOXp4klmNvU before applying your patch, both "case 1" and "case 2" end up printing does this help? |
b0ec1bd
to
0e2362f
Compare
Updated with tests. Turned out that there are special tests (apart from the normal fuzzing of types) for encoding byte arrays. |
Codecov Report
@@ Coverage Diff @@
## master #371 +/- ##
==========================================
+ Coverage 81.69% 81.81% +0.12%
==========================================
Files 41 41
Lines 5020 5021 +1
==========================================
+ Hits 4101 4108 +7
+ Misses 798 792 -6
Partials 121 121
Continue to review full report at Codecov.
|
@nikhita do we have to add the test for the |
[]byte or []uint8 are encoded as base-64 encoded string. Per this, non-nil empty slice should not get marshalled as null, rather as "". This restores compatibility with the standard library.
0e2362f
to
fb5614a
Compare
@dims Done. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. Thanks @nikhita
LGTM. I try to be judicious in my authority on this repo, but this seems like an important edge case. |
Don't marshal empty byte or uint8 slice as null
Fixes #272
[]byte
or[]uint8
are encoded as base-64 encoded strings. So, only nil slices of these types should get marshalled asnull
, and non-nil empty slices should get marshalled as""
.This restores compatibility with the standard library.
I verified that this patch works locally. Even though tests for these exist in https://github.com/json-iterator/go/blob/master/type_tests/slice_test.go, I couldn't reproduce a case where the test case was fuzzed to an empty
[]byte
or[]uint8
slice... 😕/cc @taowen @thockin
fyi @liggitt @dims @neolit123